Skip to content

Add StreamTimeoutStrategy, StreamTimeoutDecision, and Token-bucket-based timeout limiting strategy #6245

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Add StreamTimeoutStrategy, StreamTimeoutDecision, and Token-bucket-based timeout limiting strategy #6245

wants to merge 5 commits into from
+310 −55

Conversation

@sjy982
Copy link
Contributor

@sjy982 sjy982 commented May 18, 2025

Motivation

  • Pluggable timeout strategies
    StreamMessage.timeout() currently uses a fixed timeout mode. Different
    use-cases may require custom logic, so we introduce a StreamTimeoutStrategy
    interface. The existing StreamTimeoutMode will remain as the default
    strategy.
  • Token-bucket-based timeout limiting
    To tolerate transient network jitter, a token-bucket strategy can delay
    timeout until a user-defined number of consecutive delays is reached.

Modifications

  1. Add StreamTimeoutDecision
    • Immutable value object containing: timedOut, hasNextSchedule,
      nextScheduleTimeNanos.
    • Provides the factory methods:
      TIMED_OUT, NO_SCHEDULE, scheduleAt(long).
  2. Add StreamTimeoutStrategy interface
    • initialDecision(long subscribeTimeNanos) – called once right after
      subscription.
    • evaluateTimeout(long currentTimeNanos, long lastEventTimeNanos) – called
      each time the scheduled timer fires.
  3. (TODO) Default (StreamTimeoutMode) and token-bucket strategy implementations
    Will be added in a follow-up commit.

Result

@github-actions
Copy link
Contributor

github-actions bot commented May 18, 2025 •
edited
Loading

🔍 Build Scan® (commit: f80c251)

Job name Status Build Scan®
build-ubicloud-standard-16-jdk-8 ✅ https://ge.armeria.dev/s/vbsdqucj4w3vu
build-ubicloud-standard-16-jdk-21-snapshot-blockhound ✅ https://ge.armeria.dev/s/n5uy45bkomyko
build-ubicloud-standard-16-jdk-17-min-java-17-coverage ✅ https://ge.armeria.dev/s/7vpqv2cnur2sc
build-ubicloud-standard-16-jdk-17-min-java-11 ✅ https://ge.armeria.dev/s/qvuvaqyhxjef4
build-ubicloud-standard-16-jdk-17-leak ✅ https://ge.armeria.dev/s/3fbxcvbh4dly2
build-ubicloud-standard-16-jdk-11 ✅ https://ge.armeria.dev/s/stqfwqlen5k2c
build-macos-latest-jdk-21 ✅ https://ge.armeria.dev/s/2quqym4g77slc

sjy982 added 2 commits May 21, 2025 00:48
@sjy982
nextScheduleTimeoutNanos -> nextDelayNanos
71d37d8
@sjy982
Add DefaultStreamTimeoutStrategy
f80c251 
Update TimeoutStreamMessage, TimeoutSubscriber, StreamMessage
@github-actions github-actions bot added the Stale label Jun 25, 2025
@codecov
Copy link

codecov bot commented Jun 25, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 0.00%. Comparing base (8150425) to head (f80c251).
Report is 106 commits behind head on main.

Additional details and impacted files 
@@             Coverage Diff              @@
##               main   #6245       +/-   ##
============================================
- Coverage     74.46%       0   -74.47%     
============================================
  Files          1963       0     -1963     
  Lines         82437       0    -82437     
  Branches      10764       0    -10764     
============================================
- Hits          61385       0    -61385     
+ Misses        15918       0    -15918     
+ Partials       5134       0     -5134

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ikhoon ikhoon Awaiting requested review from ikhoon ikhoon will be requested when the pull request is marked ready for review ikhoon is a code owner

@jrhee17 jrhee17 Awaiting requested review from jrhee17 jrhee17 will be requested when the pull request is marked ready for review jrhee17 is a code owner

@minwoox minwoox Awaiting requested review from minwoox minwoox will be requested when the pull request is marked ready for review minwoox is a code owner

@trustin trustin Awaiting requested review from trustin trustin will be requested when the pull request is marked ready for review trustin is a code owner

Assignees

No one assigned

Labels

Stale

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support custom timeout strategies in StreamMessage.timeout(), with built-in support for a token-bucket timeout-limiting strategy

1 participant

@sjy982